Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0][IMP] shopfloor: package checkout buttons #665

Conversation

raumschmiede-joshuaL
Copy link

Adds a menu config to allow/disallow the usage of packages

Adds a menu config to allow/disallow the usage of packages
@OCA-git-bot
Copy link
Contributor

Hi @guewen, @sebalix, @simahawk,
some modules you are maintaining are being modified, check this out!

@raumschmiede-joshuaL
Copy link
Author

@mt-software-de and @jbaudoux , can also be reviewed by one of you

Copy link
Contributor

@mt-software-de mt-software-de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks.
Maybe you could rename your new created methods for a better understanding.

@@ -43,7 +43,8 @@
{
"no_prefill_qty": true,
"show_oneline_package_content": true,
"auto_post_line": true
"auto_post_line": true,
"package_process_type": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to add a migration for this change as well.
`import json
from odoo import SUPERUSER_ID, api

def post_init_hook(cr, registry):
env = api.Environment(cr, SUPERUSER_ID, {})
scenario = env.ref("shopfloor.scenario_checkout")
options = scenario.options
options["package_process_type"] = True
scenario.options_edit = json.dumps(options)
`

Comment on lines +99 to +107
def _get_allow_package_options(self):
if not self.work.menu.package_process_type:
return True, True

if self.work.menu.package_process_type == "with_package":
return True, False

return False, True

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should split this into two separate methods. This would simplify the code later.

Suggested change
def _get_allow_package_options(self):
if not self.work.menu.package_process_type:
return True, True
if self.work.menu.package_process_type == "with_package":
return True, False
return False, True
def _is_process_with_package_enabled(self):
if not self.work.menu.package_process_type:
return True
if self.work.menu.package_process_type == "with_package":
return True
return False
def _is_process_without_package_enabled(self):
if not self.work.menu.package_process_type:
return True
if self.work.menu.package_process_type == "without_package":
return True
return False

@@ -83,19 +83,28 @@ def _response_for_manual_selection(self, message=None):
return self._response(next_state="manual_selection", data=data, message=message)

def _response_for_select_package(self, picking, lines, message=None):
with_pack, wo_pack = self._get_allow_package_options()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with_pack, wo_pack = self._get_allow_package_options()

Comment on lines +93 to +94
"allow_with_package": with_pack,
"allow_without_package": wo_pack,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"allow_with_package": with_pack,
"allow_without_package": wo_pack,
"allow_with_package": self._is_process_with_package_enabled(),
"allow_without_package": self._is_process_with_package_enabled(),

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second one should be _is_process_without_package_enabled, right?

Comment on lines +939 to +955
def _check_scan_package_find_result(self, search_result):
ppt = self.work.menu.package_process_type
# Currently there is no known way to finish the checkout with a scan. To
# process without any package is only possible with a button. In its def
# a BadRequest is raised in case no package is not allowed
if not ppt or ppt == "with_package":
return

stype = search_result.type

if ppt == "without_package" and stype in [
"package",
"packaging",
"delivery_packaging",
]:
return self.msg_store.invalid_scanned_checkout_object_wo_package(stype, ppt)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a docstring and rename this method.

Suggested change
def _check_scan_package_find_result(self, search_result):
ppt = self.work.menu.package_process_type
# Currently there is no known way to finish the checkout with a scan. To
# process without any package is only possible with a button. In its def
# a BadRequest is raised in case no package is not allowed
if not ppt or ppt == "with_package":
return
stype = search_result.type
if ppt == "without_package" and stype in [
"package",
"packaging",
"delivery_packaging",
]:
return self.msg_store.invalid_scanned_checkout_object_wo_package(stype, ppt)
def _is_package_scan_allowed(self, search_result):
if self._is_process_with_package_enabled():
return
package_process_type = self.work.menu.package_process_type
if package_process_type == "without_package" and search_result.type in [
"package",
"packaging",
"delivery_packaging",
]:
return self.msg_store.checkout_package_scan_is_not_allowed(search_result.type, package_process_type)

Comment on lines +811 to +833
def invalid_scanned_checkout_object_wo_package(
self, scanned_object, package_process_type
):
scanned_object_name = {
"package": _("a package"),
"packaging": _("a packaging"),
"delivery_packaging": _("a delivery packaging"),
}.get(scanned_object, _("N/A"))

selection = self.env["shopfloor.menu"]._fields["package_process_type"].selection
ppt_name = None
for el in selection:
if el[0] == package_process_type:
ppt_name = el[1]
break

return {
"message_type": "error",
"body": _(
"You scanned {scanned_object_name} which is not allowed"
" with the package process type '{ppt_name}'"
).format(scanned_object_name=scanned_object_name, ppt_name=ppt_name),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename this to checkout_package_scan_is_not_allowed

Suggested change
def invalid_scanned_checkout_object_wo_package(
self, scanned_object, package_process_type
):
scanned_object_name = {
"package": _("a package"),
"packaging": _("a packaging"),
"delivery_packaging": _("a delivery packaging"),
}.get(scanned_object, _("N/A"))
selection = self.env["shopfloor.menu"]._fields["package_process_type"].selection
ppt_name = None
for el in selection:
if el[0] == package_process_type:
ppt_name = el[1]
break
return {
"message_type": "error",
"body": _(
"You scanned {scanned_object_name} which is not allowed"
" with the package process type '{ppt_name}'"
).format(scanned_object_name=scanned_object_name, ppt_name=ppt_name),
}
def checkout_package_scan_is_not_allowed(
self, scanned_object, package_process_type
):
scanned_object_name = {
"package": _("a package"),
"packaging": _("a packaging"),
"delivery_packaging": _("a delivery packaging"),
}.get(scanned_object, _("N/A"))
selection = self.env["shopfloor.menu"]._fields["package_process_type"].selection
ppt_name = None
for el in selection:
if el[0] == package_process_type:
ppt_name = el[1]
break
return {
"message_type": "error",
"body": _(
"You scanned {scanned_object_name} which is not allowed"
" with the package process type '{ppt_name}'"
).format(scanned_object_name=scanned_object_name, ppt_name=ppt_name),
}

@@ -913,11 +922,37 @@ def scan_package_action(self, picking_id, selected_line_ids, barcode):

selected_lines = self.env["stock.move.line"].browse(selected_line_ids).exists()
search_result = self._scan_package_find(picking, barcode)

message = self._check_scan_package_find_result(search_result)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
message = self._check_scan_package_find_result(search_result)
message = self._is_package_scan_allowed(search_result)

Comment on lines +199 to +208
package_process_type_is_possible = fields.Boolean(
compute="_compute_package_process_type_is_possible"
)
package_process_type = fields.Selection(
[
("without_package", "Without Package"),
("with_package", "With Package"),
],
"Package Proccess Type",
)
Copy link
Contributor

@jbaudoux jbaudoux Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to move the configuration to the picking type in a separate module e.g. stock_picking_putinpack_restriction in stock-logistics-workflow and handle the "put in pack" button according to the chosen option. Options similar to https://github.com/OCA/stock-logistics-warehouse/blob/14.0/stock_location_package_restriction/models/stock_location.py#L14

@raumschmiede-joshuaL
Copy link
Author

Suggested changes make sense, they will be implemented later in the future


if ppt == "without_package" and stype in [
"package",
"packaging",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"packaging",

packaging is a product packaging. This must be allowed

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product packaging can't be scanned anymore

@TDu
Copy link
Member

TDu commented Nov 17, 2023

To follow on this comment in the PR https://github.com/OCA/wms/pull/665/files#r1244810564
I have created the following:

This one should be closed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants